-
Notifications
You must be signed in to change notification settings - Fork 7
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Show loading indicator while ViaHTML iframe is loading #465
Conversation
A couple of issues in testing this: It fails to hide the spinner when loading a PDF rather than an HTML page. The background colour behind the spinner is also wrong when loading a PDF (it's set by PDF.js): I think the background colour could also be an issue with HTML pages that set a non-white background. For HTML pages also I notice that the spinner is still shown for a second on top of the page contents once they become visible: |
Instead of showing a spinner in front of the third-party page or PDF, I wonder if a better approach might be to show a loading bar above it? Either at the very top of the iframe, or above the iframe. Some sites show a simple blue line, a few pixels high, at the top of the page that moves from left to right as the page loads. The advantage of this is no clashing between our spinner and the site or PDF behind it. GitHub does this for example: The problem of course is that using a progress bar rather than a spinner requires the code to have some at least rough idea of how much of the site has loaded, not just that it's still loading. There are bar-shaped loading indicators that simply use animation to indicate that "something is happening" without indicating how much progress has been made, this sort of thing: |
I have pushed to changes to address the issues above:
Regarding the loading indicator style, I considered a GitHub-style progress bar but I thought this might be too hidden on the initial page load. On GitHub this indicator is shown when transitioning from one GH page to another so there isn't the "blank slate" problem on first load. I think we'd have to try some different variants to be sure. |
1f01cea
to
ecc5a54
Compare
An issue that is not addressed here is what to do if the Via 3 request succeeds and renders the wrapper page containing an iframe, but the ViaHTML page fails to load for any reason or renders a page without the Edit: Related issue about this: whatwg/html#125. Our use case is very similar to the ones in the comments that mention using |
Works with PDFs now 👍 There are examples of the "bar at the top of the frame" design being used in initial page loads. PDF.js does it for example (sorry long/slow GIF, wait for it): The problem is that, in Safari, the user can be faced with a blank white window and nothing happening, right? If the only thing in the blank white window is a loading bar at the top, the user will notice it. I really think that showing a spinner with transparent elements over arbitrary third-party content is a problem. It creates visual clashing and legibility issues that will vary depending on the third-party site that's behind it, and it just sort of looks like broken CSS. Example: Here's what it'll look like if the third-party site uses the same colour as we do: |
Maybe put another way: there's a reason why browsers show a tiny loading spinner up on the tab (where they have control of the background colour and surroundings), rather than a big one in the middle of the window |
@robertknight Do you want to push ahead with this design, or try out some variants? And do you want to do anything about the case when the iframe's contents fail to load? |
In Safari there is no indicator that anything is loading once the main frame has loaded, the user just sees a blank white space where the iframe is. To provide a better experience add a loading spinner in the center of the page above the iframe in all browsers. This is shown until the iframe reports the document metadata to the parent frame for the first time, corresponding to the `DOMContentLoaded` event having been received in the iframe. This is also when the tab title gets set. The loading spinner HTML and SVG have been taken from the `annotation.html.jinja2`, `bouncer.css` and `hypothesis-icon.svg` files in the hypothesis/bouncer repository. All of the resources have been inlined here, which optimizes loading a little, but we could quite easily move them to external resources in future for maintainability. Fixes hypothesis/viahtml#155
When visiting `http://<VIA_DOMAIN>/<PDF_URL>` then the PDF viewer is displayed inside an iframe. In order for the top frame rendered by the `proxy` view to reflect the document title and PDF URL the PDF viewer iframe needs to send the same `metadatachange` message that ViaHTML sends. The "real" PDF title is not available until the document has loaded, and is often incorrect anyway. Therefore we send the filename of the PDF extracted from the URL if available, or the domain from which the PDF was loaded otherwise.
- Wait for .5s before showing the spinner, to avoid an unnecessary flash if the content loads quickly - Fade the spinner in and out over 200ms to reduce the flicker effect - Change the class name from `js-loading-spinner` to a more generic `js-loading-indicator` in case we decide to change the indicator type in future (eg. from a spinner to a progress bar)
Replace the loading spinner in the center of the screen with a thin red bar at the top of the screen. This indicator is modeled after the one used by GitHub (a thin blue bar) and PDF.js (a slightly thicker white bar).
ecc5a54
to
240d3bd
Compare
I have changed the loading spinner to a solid bar that is similar to GitHub's, but using our brand color instead of blue and is slightly thicker to make it more visible. This avoids an issue where the spinner can obscure the page content if it takes a long time in between some content appearing and the Loading.bar.animation.movAs with GitHub's bar, this one doesn't use an indeterminate state during the initial load. Instead it just grows to a fixed progress percentage and stops until loading is "done" at which point it grows to 100%. To get a sense of what it looks like under different conditions, open Chrome devtools, go to the Network tab and enable connection throttling at various levels. |
*/ | ||
.loading-bar.is-done { | ||
animation: loading-bar-finish; | ||
animation-duration: 1s; |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
The animation is not actually 1s
long as it reaches the final state at 70%. Using 1s makes each 10% of the animation take 100ms, which is convenient.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Looks great!
In Safari there is no indicator that anything is loading once the main
frame has loaded, the user just sees a blank white space where the
iframe is.
To provide a better experience add a loading bar at the top of the page. This is shown until the iframe reports the document metadata to the parent frame for the first time, corresponding
to the
DOMContentLoaded
event having been received in the iframe. Thisis also when the tab title gets set. (Edited: The spinner has been changed to a loading bar)
The loading spinner HTML and SVG have been taken from the
(Edit: The spinner was replaced with a red bar.)annotation.html.jinja2
,bouncer.css
andhypothesis-icon.svg
filesin the hypothesis/bouncer repository. All of the resources have been
inlined here, which optimizes loading a little, but we could quite
easily move them to external resources in future for maintainability.
Fixes hypothesis/viahtml#155